Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Machinery to fix total issuance accounting and keep it fixed #595

Closed
wants to merge 7 commits into from
Closed

Machinery to fix total issuance accounting and keep it fixed #595

wants to merge 7 commits into from

Conversation

orriin
Copy link
Contributor

@orriin orriin commented Jun 28, 2024 •
edited

DO NOT MERGE UNTIL CONST HAS APPROVED

Machinery for #563

  • Adds documentation for pallet_subtensor::TotalIssuance
  • Introduces new storage item TotalSubnetLocked to track the total amount of tokens locked for subnet registration
  • Introduces migration to initialize TotalSubnetLocked and then rejig the total issuance to be the sum of issued tokens, staked tokens, and tokens locked for subnet registration
  • Adjusts set_subnet_locked_balance to update TotalSubnetLocked to reflect changes in SubnetLocked
  • Creates custom on_nonzero_unbalanced handler for transaction fees, which will remove the burnt transaction fee amount from pallet_subtensor::TotalIssuance before finally dropping it
    • Tested on localnet
  • Adds try-state checks for
    • TotalStake
    • TotalSubnetLock
    • TotalIssuance

@orriin orriin added the blue team defensive programming, CI, etc label Jun 28, 2024
@orriin orriin requested a review from open-junius June 28, 2024 11:51
open-junius
open-junius previously approved these changes Jun 28, 2024
View reviewed changes
@orriin @camfairchild
machinery to fix total issuance
f74d800 
comment

tx fee handler

Update pallets/subtensor/src/lib.rs

Co-authored-by: Cameron Fairchild <cameron@opentensor.ai>
@orriin orriin mentioned this pull request Jul 1, 2024
Closed
@orriin @camfairchild
Update pallets/subtensor/src/lib.rs
fd27e9d 
Co-authored-by: Cameron Fairchild <cameron@opentensor.ai>
camfairchild
camfairchild reviewed Jul 15, 2024
View reviewed changes
Copy link
Contributor

@camfairchild camfairchild left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a test for the total issuance fees hook? If not, that would be great to add.

pallets/subtensor/src/utils.rs Outdated Show resolved Hide resolved
@orriin
Copy link
Contributor Author

orriin commented Jul 17, 2024

Do we have a test for the total issuance fees hook? If not, that would be great to add.

It will be tested every pr with live state in the existing try-runtime tests

open-junius
open-junius previously approved these changes Jul 17, 2024
View reviewed changes
camfairchild
camfairchild previously approved these changes Jul 17, 2024
View reviewed changes
sam0x17
sam0x17 previously approved these changes Jul 18, 2024
View reviewed changes
@orriin orriin dismissed stale reviews from sam0x17, camfairchild, and open-junius via b34c052 July 22, 2024 13:18
distributedstatemachine
distributedstatemachine requested changes Jul 23, 2024
View reviewed changes
pallets/subtensor/src/lib.rs Show resolved Hide resolved
runtime/src/migrations/initialise_ti.rs Outdated Show resolved Hide resolved
distributedstatemachine
distributedstatemachine requested changes Jul 23, 2024
View reviewed changes
Copy link
Contributor

@distributedstatemachine distributedstatemachine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add tests for the migrations ?

pallets/admin-utils/src/lib.rs Outdated Show resolved Hide resolved
pallets/admin-utils/tests/tests.rs Outdated Show resolved Hide resolved
pallets/admin-utils/tests/tests.rs Outdated Show resolved Hide resolved
@unconst unconst closed this Jul 23, 2024
@orriin orriin reopened this Jul 24, 2024
@orriin orriin requested a review from unconst as a code owner July 24, 2024 13:44
@orriin
Copy link
Contributor Author

orriin commented Jul 24, 2024

DO NOT MERGE UNTIL CONST HAS APPROVED

@orriin
initialise total issuance with a migration
64993cc 
initialise total issuance with a migration
@orriin orriin closed this Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@distributedstatemachine distributedstatemachine distributedstatemachine requested changes

@sam0x17 sam0x17 sam0x17 left review comments

@camfairchild camfairchild camfairchild left review comments

@open-junius open-junius open-junius left review comments

@unconst unconst Awaiting requested review from unconst unconst is a code owner

Assignees
No one assigned
Labels
blue team defensive programming, CI, etc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@orriin @sam0x17 @camfairchild @distributedstatemachine @open-junius @unconst